Skip to content

[Fefactor and Fix] Refactor v0.0.3 and fix CLI issues#2

Merged
wangxingjun778 merged 15 commits into
mainfrom
fix/v3_reviews
Jun 4, 2026
Merged

[Fefactor and Fix] Refactor v0.0.3 and fix CLI issues#2
wangxingjun778 merged 15 commits into
mainfrom
fix/v3_reviews

Conversation

@wangxingjun778
Copy link
Copy Markdown
Member

@wangxingjun778 wangxingjun778 commented Jun 4, 2026

  • CLI Command Promotion: Promoted create, info, list, and delete to top-level commands.
  • Backward Compatibility: Retained the legacy repo command as a hidden alias (implemented via help=argparse.SUPPRESS for stability).
  • Dataset Routing Fixes: Corrected dataset operations to properly route to multipart and /repo/tree endpoints.
  • API Enum Support: Updated endpoint checks in _legacy_api.py to properly handle the RepoType.DATASET enum.
  • Enhanced Error Reporting: Added request and response context to error messages for easier debugging.
  • Test Coverage: Added comprehensive mock-based unit tests.

wangxingjun778 and others added 13 commits June 3, 2026 14:51
…listing

Root cause: ModelScope API uses /repo/tree (not /repo/files) for datasets.
The /repo/files endpoint returns 405 for dataset repos.

Also:
- Add HTTP 405 → InvalidParameter mapping in _STATUS_MAP
- Enrich APIError fallback message with request method/path for diagnostics
- Route list_repo_files to /repo/tree when repo_type is dataset

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ression

Prior tests only covered model repos, missing the critical difference that
datasets use /repo/tree (not /repo/files) for file listing. This caused a
405 error on dataset download that went undetected.

Tests cover: list_repo_files, download_file, download_repo, and CLI download
for dataset repos using a known public dataset (no auth needed).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
MCP repos cannot be created via the SDK (no API endpoint exists).
Previously this fell through to POST /api/v1/mcps → 404.

- Add _CREATABLE_TYPES validation in create_repo → raises NotSupportedError
- Restrict CLI `ms repo create --repo-type` choices to supported types

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously: ms repo create/info/list/delete
Now: ms create/info/list/delete (with --repo-type)

- Promote repo leaf commands to top-level (CreateCommand, InfoCommand, etc.)
- Keep `ms repo <action>` as hidden backward-compat alias
- Update README CLI reference and examples
- Update tests to use new command form

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…mands

Add parser + execution (mock HubApi) tests for every CLI subcommand,
increasing coverage from ~57 to 299 unit tests running in ~1.5s with
zero network access. Remove 27 duplicate tests from test_compat.py,
fix unused imports, replace fragile private-method patching in download
tests, and use real UserInfo dataclass instead of MagicMock.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rror messages

- Dataset creation aligned with upstream server: POST /api/v1/datasets
  requires multipart form data (files=) not JSON, and uses "Owner" key
  instead of "Path" for namespace
- Added files parameter to LegacyClient._request() for multipart support
- API error __str__ now includes server error code and detail fields from
  response body instead of hiding them
- Added 23 new CLI unit tests covering previously untested execution paths
- Fixed README: --log-type runtime → run (4 occurrences)
- Remote test filtering now env-controlled via MODELSCOPE_RUN_REMOTE_TESTS

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- errors.py: APIError gains url/method attrs, __str__ appends debug context
- _legacy_api.py: add DEBUG log on HTTP >= 400 with method/url/status/body
- _openapi.py: same DEBUG logging on failure
- Response body truncated to 500 chars, no token leakage
- conftest.py: add MODELSCOPE_RUN_REMOTE_TESTS control, mock_only marker
- All mock execution tests marked @pytest.mark.mock_only
- Remote tests default when credentials present, mock when explicitly disabled
- Add dataset create tests: CLI execution (3), remote lifecycle (4), SDK CRUD (1)
- Total: 405 tests = 230 local + 93 mock_only + 82 remote
- Refactor make_api() to auto-merge subcmd_token/subcmd_endpoint,
  eliminating manual _merge_subcmd_auth() calls in every command
- Add --token/--endpoint to all commands that were missing it:
  repo (create/info/list/delete), deploy/stop/logs/settings,
  secret (list/add/update/delete), mcp (list/info/deploy/undeploy),
  whoami
- Restrict ms download --repo-type to {model,dataset} per README spec
- Improve NotExistError suggestion to hint about private repo auth
- Normalize visibility field from OpenAPI private/gated bools
- Update README: document --token/--endpoint placement flexibility
- Add 14 parser tests verifying --token/--endpoint across commands

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Studios are application containers without a file listing API.
Raise NotSupportedError in download_file() and download_repo() for
repo_type=studio, guiding users to git clone instead.
README now explicitly documents which --repo-type values each command
accepts, with notes about studio-only limitations. Tests parametrized
to verify rejection of all invalid repo types.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The server's POST /studios and PATCH /studios/.../settings endpoints
expect the camelCase field name `coverImage`, not `cover_image`. Sending
the snake_case variant caused "更新模型失败" errors for any non-empty value.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the ModelScope Hub CLI to promote commands like create, info, list, and delete to top-level commands while preserving the legacy repo command as a hidden alias. It also introduces extensive unit tests using mocks, improves error reporting with request/response context, and fixes dataset-specific operations by routing them to the correct multipart and /repo/tree endpoints. Feedback on the changes highlights two key areas: first, modifying the private _choices_actions attribute in argparse is fragile and should be replaced with help=argparse.SUPPRESS; second, the dataset endpoint check in _legacy_api.py should include the RepoType.DATASET enum to prevent incorrect endpoint routing when the enum is passed.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/modelscope_hub/cli/repo.py
Comment thread src/modelscope_hub/_legacy_api.py Outdated
wangxingjun778 and others added 2 commits June 4, 2026 16:51
1. Wrap _choices_actions manipulation in try/except for robustness
   (argparse.SUPPRESS doesn't reliably hide subparsers in Python 3.13+)

2. Include RepoType.DATASET enum in the repo/tree endpoint check in
   _legacy_api.py to handle callers passing the enum directly

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@wangxingjun778 wangxingjun778 merged commit 1ce1402 into main Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant